Make fileActions mixin standalone again. #6453
Conversation
|
Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes. |
dschmidt
force-pushed
the
make_fileactions_mixin_standalone
branch
from
fe1ef1f
to
a07f09f
Compare
In #6445 we introduced a dependency on isFilesAppActive for the fileActions mixin. We added it to ContextActions and BatchActions, but there are way more places where the mixin is used. That's why we add a computed to the mixin itself now. Because mixins belong to the options api we unfortunately cannot use composables directly and extract helper functions for that reason.
dschmidt
force-pushed
the
make_fileactions_mixin_standalone
branch
from
a07f09f
to
5920d71
Compare
| @@ -44,6 +39,11 @@ export default { | |||
| Move, | |||
| Restore | |||
| ], | |||
| setup() { | |||
| return { | |||
| isFilesAppActive: useIsFilesAppActive() | |||
There was a problem hiding this comment.
I'm confused, can't you delete this again with the helper function now being used in the mixins?
| @@ -55,6 +58,12 @@ export default { | |||
| ...mapGetters('Files', ['highlightedFile', 'currentFolder']), | |||
| ...mapGetters(['capabilities', 'configuration']), | |||
|
|
|||
| // TODO: Replace this with the useIsFilesAppActive composable | |||
| // once we port this mixin to something using the composition api | |||
| isFilesAppActive() { | |||
There was a problem hiding this comment.
Oh wow, is it really sufficient to provide the function here? Are we using the fileActions mixin really in all locations that use any of the specific file action mixins?
There was a problem hiding this comment.
That's why the composable is still used in BatchActions ...
We include single actions there and not the fileActions mixin.
... but that's a very good point, this has become hard to reason about. I will add this to all the mixins :(
…n that needs it. It's very hard to figure out if dependencies are provided to mixins in every single location they are used in, hence we remove the external dependency.
|
SonarCloud Quality Gate failed. |
|
Results for oCISSharingInternal2 https://drone.owncloud.com/owncloud/web/22895/60/1
|
|
Results for oCISTrashbinUploadMoveJourney https://drone.owncloud.com/owncloud/web/22895/69/1
|
delete-merged-branch
bot
deleted the
make_fileactions_mixin_standalone
branch
Description
In #6445 we introduced a dependency on isFilesAppActive for the fileActions mixin. We added it to
ContextActions and BatchActions, but there are way more places where the mixin is used. That's why
we add a computed to the mixin itself now. Because mixins belong to the options api we unfortunately
cannot use composables directly and extract helper functions for that reason.
Related Issue
Motivation and Context
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist:
Open tasks: